Skip to content

Conversation

@kreys
Copy link
Contributor

@kreys kreys commented Oct 27, 2025

Problem

Serializing AssetReferenceSprite caused infinite recursion:
RuntimeType → RuntimeModule → RuntimeAssembly → RuntimeModule → ...

Result: OutOfMemoryException, massive log spam, crash.

Solution

Filter System.Reflection types from GetSerializableFields() and GetSerializableProperties():

  • System.Reflection.* namespace
  • System.Type, System.RuntimeType
  • RuntimeModule, RuntimeAssembly

Why Correct

Reflection metadata:

  • Cannot be reconstructed from JSON
  • Provides no useful information for AI analysis
  • Should never be part of serialization/deserialization cycle
  • Is runtime-specific CLR implementation detail

Impact

✅ Fixes OutOfMemoryException crashes
✅ Cleaner JSON output without metadata noise
✅ No breaking changes - filters unsupported types only

kreys and others added 2 commits October 27, 2025 10:09
Filter out System.Reflection types (RuntimeType, RuntimeModule, RuntimeAssembly)
from serialization to prevent circular references causing OutOfMemoryException
when reading assets with AssetReferences.

Reflection metadata cannot be meaningfully serialized to JSON or reconstructed,
so excluding it improves both stability and output quality for read/write operations.
Copilot AI review requested due to automatic review settings October 27, 2025 22:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes an infinite recursion bug in the Unity MCP Plugin's serialization system that occurred when serializing Unity assets containing System.Reflection types. The fix prevents OutOfMemoryException crashes by filtering out reflection metadata types (RuntimeType, RuntimeModule, RuntimeAssembly, etc.) from serialization, as these types form circular reference chains and provide no useful information for AI analysis.

Key changes:

  • Added IsReflectionType() helper method to identify and filter System.Reflection namespace types
  • Applied reflection type filtering to both field and property serialization in converter classes
  • Added null-check guard when adding components to prevent silent failures

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
UnityGenericReflectionConvertor.cs Implements reflection type filtering in GetSerializableFields() and GetSerializableProperties() with new IsReflectionType() helper
UnityArrayReflectionConvertor.cs Applies identical reflection type filtering logic to array converter
GameObject.AddComponent.cs Adds null-check validation after AddComponent() to warn about failures

@kreys
Copy link
Contributor Author

kreys commented Oct 27, 2025

I've addressed feedback issues

Copy link
Owner

@IvanMurzak IvanMurzak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job overall!
Need to make it in a bit different way.
Need to add a custom ReflectionConvertor for those types which create infinite recursion issue.

I described everything in this two videos:

// Catch System.Reflection.* namespace types
// Note: System.RuntimeType and System.Type are in System namespace, not System.Reflection
return fullName.StartsWith("System.Reflection.") ||
fullName == "System.RuntimeType" ||
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change it to type comparison instead of string comparison

type == typeof(System.RuntimeType)
type == typeof(System.Type)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System.RuntimeType is not a real type


// Catch System.Reflection.* namespace types
// Note: System.RuntimeType and System.Type are in System namespace, not System.Reflection
return fullName.StartsWith("System.Reflection.") ||
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure sure about ignoring entire namespace System.Reflection.*. Any purposes of this?

Comment on lines +27 to +36
.Where(field => field.IsPublic || field.IsPrivate && field.GetCustomAttribute<SerializeField>() != null)
.Where(field => !ReflectionTypeFilter.IsReflectionType(field.FieldType));

public override IEnumerable<PropertyInfo>? GetSerializableProperties(Reflector reflector, Type objType, BindingFlags flags, ILogger? logger = null)
{
var baseProperties = base.GetSerializableProperties(reflector, objType, flags, logger);
if (baseProperties == null) return null;

return baseProperties.Where(prop => !ReflectionTypeFilter.IsReflectionType(prop.PropertyType));
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to do it in this class. Need to take care of the problematic types globally by making a custom ReflectionConvertors for that type.

Comment on lines +27 to +36
.Where(field => field.IsPublic || field.IsPrivate && field.GetCustomAttribute<SerializeField>() != null)
.Where(field => !ReflectionTypeFilter.IsReflectionType(field.FieldType));

public override IEnumerable<PropertyInfo>? GetSerializableProperties(Reflector reflector, Type objType, BindingFlags flags, ILogger? logger = null)
{
var baseProperties = base.GetSerializableProperties(reflector, objType, flags, logger);
if (baseProperties == null) return null;

return baseProperties.Where(prop => !ReflectionTypeFilter.IsReflectionType(prop.PropertyType));
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to do it in this class. Need to take care of the problematic types globally by making a custom ReflectionConvertors for that type.

@IvanMurzak IvanMurzak added the bug Something isn't working label Oct 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants